Skip to content

[WIP] server: fix checking disk offering access for snapshot volume#3786

Closed
shwstppr wants to merge 1 commit into
apache:masterfrom
shapeblue:fix-volume-pure-snapshot-npe
Closed

[WIP] server: fix checking disk offering access for snapshot volume#3786
shwstppr wants to merge 1 commit into
apache:masterfrom
shapeblue:fix-volume-pure-snapshot-npe

Conversation

@shwstppr
Copy link
Copy Markdown
Contributor

@shwstppr shwstppr commented Dec 24, 2019

Fixes #3783
As reported in the issue, creating volumes from pure snapshot fails with NPE. This is due to order of calls where disk offering access is checked before checking disk offering value. This PR fixes the same.

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

How Has This Been Tested?

In progress

…e NPE

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@shwstppr shwstppr changed the title [WIP] server: fix checking disk offering access for pure snapshot volume [WIP] server: fix checking disk offering access for snapshot volume Dec 24, 2019
@shwstppr
Copy link
Copy Markdown
Contributor Author

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@shwstppr a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

Packaging result: ✖centos6 ✔centos7 ✔debian. JID-507

@DaanHoogland
Copy link
Copy Markdown
Contributor

@shwstppr code looks good, but the bug existed IMHO because of the method being 200 lines long and thus illegible. I will let this pass to honour history but if you have time please factor out the method contents to a helper class or a set of helper methods?
Also, i think we want this on 4.13.
can you give a second look? tnx.

@shwstppr
Copy link
Copy Markdown
Contributor Author

shwstppr commented Jan 2, 2020

Closing in favour of #3791
@DaanHoogland will address long method refactoring changes there

@shwstppr shwstppr closed this Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create volume or template from snapshot is broken

4 participants